-
Notifications
You must be signed in to change notification settings - Fork 66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CAN: Loopback() #286
CAN: Loopback() #286
Conversation
Todo: comments some part can be improved...
Thanks LJ! I can't review a lot here. I could test it of course, if you describe the test setup. For me it's fine to have this WiP on the feature branch. Can you create follow-up tickets for the open todos? |
@@ -0,0 +1,61 @@ | |||
// Copyright (c) Linjing Zhang. All rights reserved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Big topic this header about the licensing :)
We can talk about it.
libraries/CAN/src/CANXMC.cpp
Outdated
@@ -202,14 +220,63 @@ CANXMC::~CANXMC() {} | |||
|
|||
int CANXMC::observe() | |||
{ | |||
XMC_CAN_NODE_SetInitBit(_XMC_CAN_config->can_node); | |||
// TOOD: LJ: no idea what this is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tood? xD
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol I will fix it
libraries/CAN/src/CANXMC.cpp
Outdated
XMC_CAN_Enable(CAN_xmc); | ||
/* Configuration of CAN Node and enable the clock */ | ||
XMC_CAN_InitEx(CAN_xmc, _XMC_CAN_config->can_clock , _XMC_CAN_config->can_frequency); | ||
if (XMC_CAN_STATUS_SUCCESS == XMC_CAN_NODE_NominalBitTimeConfigureEx( CAN_NODE1, &CAN_NODE_bit_time_config)){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in general, keep that in mind for the whole file. We don´t have a formating tool yet, but at least within a file we can keep some consistancy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I formatted locally using clang style.
I can also add this: https://github.com/marketplace/actions/clang-format-code-formatter for style checking. Is it ok?
That way we don't get PRs with a lot of formatting changes... like the opened one...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
topic close.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so far looks good to me.
Hi Julian, for loopback you need no extra hardware setup. You could test it with a single XMC1400 2Go. I did not test it on XMC4000 series, maybe you can do that. For The namespace, I talked once with Jens (related ticket: https://jirard.intra.infineon.com/browse/DESMAKERS-3828), namespace offers several benefits, for example avoiding naming conflicts with third-party libraries. I also referenced this post: https://luckyresistor.me/2014/10/31/how-and-why-to-use-namespaces/ |
If you tested it on the 2Go it's fine for the feature branch. Regarding the namespace it's a matter of hw abstraction which should not be handled in the application code. I would like to allow our device to work also with third party code and vice versa. But I can check with Jens next week :) For me it's fine to merge this to the feature branch if Juan and Olaf agree. |
I will first close this ticket and move ifx namespace in next PR. |
By creating this pull request you agree to the terms in CONTRIBUTING.md.
https://github.com/Infineon/.github/blob/master/CONTRIBUTING.md
--- DO NOT DELETE ANYTHING ABOVE THIS LINE ---
CONTRIBUTING.md also tells you what to expect in the PR process.
Description
Add loopback functionalities for CAN lib.
Related Issue
NA
Context
Add loopback() function.
Basically writing application code instead of a library... but no idea how to do better using XMCLib.